fsck: Implement a partial commit reason bitmask

After the corruption has been fixed with "ostree fsck -a --delete", a
second run of the "ostree fsck" command will print X partial commits
not verified and exit with a zero.

The zero exit code makes it hard to detect if a repair operation needs
to be run.  When ever fsck creates a partial commit it should add a
reason for the partial commit to the state file found in
state/<hash>.commitpartial.  This will allow a future execution of the
fsck to still return an error indicating that the repository is still
in the damaged state, awaiting repair.

Additional reason codes could be added in the future for why a partial
commit exists.

Text from: https://github.com/ostreedev/ostree/pull/1880
====
cgwalters commented:

To restate, the core issue is that it's valid to have partial commits
for reasons other than fsck pruned bad objects, and libostree doesn't
have a way to distinguish.

Another option perhaps is to write e.g. fsck-partial into the
statefile state/<hash>.commitpartial which would mean "partial, and
expected to exist but was pruned by fsck" and fsck would continue to
error out until the commit was re-pulled. Right now the partial stamp
file is empty, so it'd be fully compatible to write a rationale into
it.
====

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Closes: #1910
Approved by: cgwalters

diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt
index e8faeb10dfadec9d4c16baea78337a89d42e75a2..252a563acba7216cb3d344ea261f5b908d52a4a7 100644 (file)
--- a/apidoc/ostree-sections.txt
+++ b/apidoc/ostree-sections.txt
@@ -333,6 +333,7 @@ ostree_repo_set_cache_dir
 ostree_repo_sign_delta
 ostree_repo_has_object
 ostree_repo_mark_commit_partial
+ostree_repo_mark_commit_partial_reason
 ostree_repo_write_metadata
 ostree_repo_write_metadata_async
 ostree_repo_write_metadata_finish

diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym
index f552bcea0b1ba899337102cf8b18f7db6a8063f1..58d35f5556b08ff264c4a9dbfbc9318c5466f589 100644 (file)
--- a/src/libostree/libostree-devel.sym
+++ b/src/libostree/libostree-devel.sym
@@ -19,6 +19,7 @@
 
 /* Add new symbols here.  Release commits should copy this section into -released.sym. */
 LIBOSTREE_2019.4 {
+  ostree_repo_mark_commit_partial_reason;
 } LIBOSTREE_2019.3;
 
 /* Stub section for the stable release *after* this development one; don't

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index e7bc98207bc44a3e42f5fc8e47d65d532433d321..0aaebbdb58ad480287e04aa749c4a1e5bcddef0b 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -1874,26 +1874,29 @@ ensure_txn_refs (OstreeRepo *self)
 }
 
 /**
- * ostree_repo_mark_commit_partial:
+ * ostree_repo_mark_commit_partial_reason:
  * @self: Repo
  * @checksum: Commit SHA-256
  * @is_partial: Whether or not this commit is partial
+ * @in_state: Reason bitmask for partial commit
  * @error: Error
  *
- * Commits in "partial" state do not have all their child objects written. This
- * occurs in various situations, such as during a pull, but also if a "subpath"
- * pull is used, as well as "commit only" pulls.
+ * Allows the setting of a reason code for a partial commit. Presently
+ * it only supports setting reason bitmask to
+ * OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL, or
+ * OSTREE_REPO_COMMIT_STATE_NORMAL.  This will allow successive ostree
+ * fsck operations to exit properly with an error code if the
+ * repository has been truncated as a result of fsck trying to repair
+ * it.
  *
- * This function is used by ostree_repo_pull_with_options(); you
- * should use this if you are implementing a different type of transport.
- *
- * Since: 2017.15
+ * Since: 2019.4
  */
 gboolean
-ostree_repo_mark_commit_partial (OstreeRepo     *self,
-                                 const char     *checksum,
-                                 gboolean        is_partial,
-                                 GError        **error)
+ostree_repo_mark_commit_partial_reason (OstreeRepo     *self,
+                                        const char     *checksum,
+                                        gboolean        is_partial,
+                                        OstreeRepoCommitState in_state,
+                                        GError        **error)
 {
   g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum);
   if (is_partial)
@@ -1905,6 +1908,12 @@ ostree_repo_mark_commit_partial (OstreeRepo     *self,
           if (errno != EEXIST)
             return glnx_throw_errno_prefix (error, "open(%s)", commitpartial_path);
         }
+      else
+        {
+          if (in_state & OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL)
+            if (glnx_loop_write (fd, "f", 1) < 0)
+              return glnx_throw_errno_prefix (error, "write(%s)", commitpartial_path);
+        }
     }
   else
     {
@@ -1915,6 +1924,34 @@ ostree_repo_mark_commit_partial (OstreeRepo     *self,
   return TRUE;
 }
 
+/**
+ * ostree_repo_mark_commit_partial:
+ * @self: Repo
+ * @checksum: Commit SHA-256
+ * @is_partial: Whether or not this commit is partial
+ * @error: Error
+ *
+ * Commits in the "partial" state do not have all their child objects
+ * written.  This occurs in various situations, such as during a pull,
+ * but also if a "subpath" pull is used, as well as "commit only"
+ * pulls.
+ *
+ * This function is used by ostree_repo_pull_with_options(); you
+ * should use this if you are implementing a different type of transport.
+ *
+ * Since: 2017.15
+ */
+gboolean
+ostree_repo_mark_commit_partial (OstreeRepo     *self,
+                                 const char     *checksum,
+                                 gboolean        is_partial,
+                                 GError        **error)
+{
+  return ostree_repo_mark_commit_partial_reason (self, checksum, is_partial,
+                                                 OSTREE_REPO_COMMIT_STATE_NORMAL,
+                                                 error);
+}
+
 /**
  * ostree_repo_transaction_set_refspec:
  * @self: An #OstreeRepo

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index eb652bef0b0b8a715e818970e26fc2973fcc7328..b654aff2a0b833511d8f4029b11dc2ff6da54e70 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3768,10 +3768,19 @@ load_metadata_internal (OstreeRepo       *self,
           g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (sha256);
           *out_state = 0;
 
-          if (!glnx_fstatat_allow_noent (self->repo_dir_fd, commitpartial_path, NULL, 0, error))
+          glnx_autofd int fd = -1;
+          if (!ot_openat_ignore_enoent (self->repo_dir_fd, commitpartial_path, &fd, error))
             return FALSE;
-          if (errno == 0)
-            *out_state |= OSTREE_REPO_COMMIT_STATE_PARTIAL;
+          if (fd != -1)
+            {
+              *out_state |= OSTREE_REPO_COMMIT_STATE_PARTIAL;
+               char reason;
+               if (read (fd, &reason, 1) == 1)
+                 {
+                   if (reason == 'f')
+                     *out_state |= OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL;
+                 }
+            }
         }
     }
   else if (self->parent_repo)

diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h
index 038bbd411f8f90cebb3191be1838ae0e379d5067..aaaaa622c6b58fd97e75b357ae2ef9880da79a51 100644 (file)
--- a/src/libostree/ostree-repo.h
+++ b/src/libostree/ostree-repo.h
@@ -248,6 +248,26 @@ gboolean      ostree_repo_write_config (OstreeRepo *self,
                                         GKeyFile   *new_config,
                                         GError    **error);
 
+/**
+ * OstreeRepoCommitState:
+ * @OSTREE_REPO_COMMIT_STATE_NORMAL: Commit is complete. This is the default.
+ *    (Since: 2017.14.)
+ * @OSTREE_REPO_COMMIT_STATE_PARTIAL: One or more objects are missing from the
+ *    local copy of the commit, but metadata is present. (Since: 2015.7.)
+ * @OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL: One or more objects are missing from the
+ *    local copy of the commit, due to an fsck --delete. (Since: 2019.3.)
+ *
+ * Flags representing the state of a commit in the local repository, as returned
+ * by ostree_repo_load_commit().
+ *
+ * Since: 2015.7
+ */
+typedef enum {
+  OSTREE_REPO_COMMIT_STATE_NORMAL = 0,
+  OSTREE_REPO_COMMIT_STATE_PARTIAL = (1 << 0),
+  OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL = (1 << 1),
+} OstreeRepoCommitState;
+
 /**
  * OstreeRepoTransactionStats:
  * @metadata_objects_total: The total number of metadata objects
@@ -315,6 +335,13 @@ gboolean      ostree_repo_mark_commit_partial (OstreeRepo     *self,
                                                gboolean        is_partial,
                                                GError        **error);
 
+_OSTREE_PUBLIC
+gboolean      ostree_repo_mark_commit_partial_reason (OstreeRepo     *self,
+                                                      const char     *checksum,
+                                                      gboolean        is_partial,
+                                                      OstreeRepoCommitState in_state,
+                                                      GError        **error);
+
 _OSTREE_PUBLIC
 void          ostree_repo_transaction_set_refspec (OstreeRepo *self,
                                                    const char *refspec,
@@ -526,23 +553,6 @@ gboolean      ostree_repo_load_variant_if_exists (OstreeRepo  *self,
                                                   GVariant     **out_variant,
                                                   GError       **error);
 
-/**
- * OstreeRepoCommitState:
- * @OSTREE_REPO_COMMIT_STATE_NORMAL: Commit is complete. This is the default.
- *    (Since: 2017.14.)
- * @OSTREE_REPO_COMMIT_STATE_PARTIAL: One or more objects are missing from the
- *    local copy of the commit, but metadata is present. (Since: 2015.7.)
- *
- * Flags representing the state of a commit in the local repository, as returned
- * by ostree_repo_load_commit().
- *
- * Since: 2015.7
- */
-typedef enum {
-  OSTREE_REPO_COMMIT_STATE_NORMAL = 0,
-  OSTREE_REPO_COMMIT_STATE_PARTIAL = (1 << 0),
-} OstreeRepoCommitState;
-
 _OSTREE_PUBLIC
 gboolean      ostree_repo_load_commit (OstreeRepo            *self,
                                        const char            *checksum, 

diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c
index a7ecd3d0c30310978187f9f4a3bd67f888929f27..44e98a763716ab56a707fe8361b73527a24d4c68 100644 (file)
--- a/src/ostree/ot-builtin-fsck.c
+++ b/src/ostree/ot-builtin-fsck.c
@@ -127,7 +127,7 @@ fsck_one_object (OstreeRepo            *repo,
                   if ((state & OSTREE_REPO_COMMIT_STATE_PARTIAL) == 0)
                     {
                       g_printerr ("Marking commit as partial: %s\n", parent_commit);
-                      if (!ostree_repo_mark_commit_partial (repo, parent_commit, TRUE, error))
+                      if (!ostree_repo_mark_commit_partial_reason (repo, parent_commit, TRUE, OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL, error))
                         return FALSE;
                     }
                 }
@@ -302,6 +302,7 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation,
     opt_verify_bindings = TRUE;
 
   guint n_partial = 0;
+  guint n_fsck_partial = 0;
   g_hash_table_iter_init (&hash_iter, objects);
   while (g_hash_table_iter_next (&hash_iter, &key, &value))
     {
@@ -410,7 +411,11 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation,
             }
 
           if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL)
-            n_partial++;
+            {
+              n_partial++;
+              if (commitstate & OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL)
+                n_fsck_partial++;
+            }
           else
             g_hash_table_add (commits, g_variant_ref (serialized_key));
         }
@@ -450,5 +455,8 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation,
   if (found_corruption)
     return glnx_throw (error, "Repository corruption encountered");
 
+  if (n_fsck_partial > 0)
+    return glnx_throw (error, "%u fsck deleted partial commits not verified", n_partial);
+
   return TRUE;
 }